Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Some changes occurred in compiler/rustc_attr_data_structures

cc @jdonszelmann

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann

☔ The latest upstream changes (presumably #140717) made this pull request unmergeable. Please resolve the merge conflicts.

@Periodic1911 if you rebase I'll run a crater

@rust-lang/lang this makes a long-standing warning an error with a crater run. Just wanted to notify you

@bors try

⌛ Trying commit ac1f122 with merge 723ab94...

☀️ Try build successful - checks-actions
Build commit: 723ab94 (723ab942d877517caf445dafde0416803ef56501)

@craterbot check

👌 Experiment pr-143857 created and queued.
🤖 Automatically detected try build 723ab94
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

🚧 Experiment pr-143857 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

🎉 Experiment pr-143857 is completed!
📊 23 regressed and 8 fixed (664032 total)
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

So the crater run confirmed two breaking changes, the invalid_macro_export_arguments lint and #[macro_export()].

All invalid attribute arguments are being turned into errors so this lint which is specific to the macro_export attribute will be redundant. The crater run shows that no one is using invalid arguments to macro_export, and no one is setting the lint to allowed. The invalid_macro_export_arguments lint is used by four crates, all belonging to the same author, daiwanwei, and in all cases it is used in the Cargo.toml to deny the lint, which is effectively the same as making it an error. I propose that I will send a pr to daiwanwei's crates to not use the lint and then we can remove it from rustc. Alternatively, we can leave an empty lint behind.

#[macro_export()] with the () is accepted by stable Rust with no warning, although the correct syntax would be #[macro_export]. #[macro_export()] is used be nine crates. I propose that I will make #[macro_export()] a warning with a future error.

@rust-lang/lang what do you think about my proposal for how to handle these breaking changes?

@Periodic1911 For the lint removal, we have a system for listing removed lints to be silently ignored, where it'll only cost one line in an array. We should add it there (when we're ready to remove it), rather than making the breaking change of removing it.

The crater run shows that no one is using invalid arguments to macro_export, and no one is setting the lint to allowed.

I saw this, though? https://crater-reports.s3.amazonaws.com/pr-143857/try%23723ab942d877517caf445dafde0416803ef56501/gh/WillSilva83.DataFakerCreatorRust/log.txt

[INFO] [stdout] error[E0539]: malformed `macro_export` attribute input
[INFO] [stdout]    --> /opt/rustwide/cargo-home/registry/src/index.crates.io-1949cf8c6b5b557f/polars-plan-0.27.2/src/dsl/function_expr/mod.rs:239:1
[INFO] [stdout]     |
[INFO] [stdout] 239 | #[macro_export(super)]
[INFO] [stdout]     | ^^^^^^^^^^^^^^^-----^^
[INFO] [stdout]     |                |
[INFO] [stdout]     |                the only valid argument here is `local_inner_macros`
[INFO] [stdout]     |
[INFO] [stdout] help: try changing it to one of the following valid forms of the attribute
[INFO] [stdout]     |
[INFO] [stdout] 239 - #[macro_export(super)]
[INFO] [stdout] 239 + #[macro_export(local_inner_macros)]
[INFO] [stdout]     |
[INFO] [stdout] 239 - #[macro_export(super)]
[INFO] [stdout] 239 + #[macro_export]
[INFO] [stdout]     |

We discussed this in the lang team triage meeting. We agreed on a plan to upgrade invalid_macro_export_arguments to FCW showing in dependencies, and also upgrade it to deny by default.

If and when we upgrade this lint to a hard error, we can add it to the removed_lints list and that should handle it correctly.

FCP to ratify this plan:

@rfcbot fcp merge

Team member @tmandry has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rfcbot reviewed

🔔 This is now entering its final comment period, as per the review above. 🔔

Oh thanks scottmcm, I didn't know the dependencies tab showed results that aren't in the root results tab. Sorry, I'm new to this! 🙇‍♀️ And thank you for checking my work